Skip to content

Comments

Exclude invited users from User Joined#1602

Merged
riderx merged 5 commits intomainfrom
riderx/user-joined-metric
Feb 9, 2026
Merged

Exclude invited users from User Joined#1602
riderx merged 5 commits intomainfrom
riderx/user-joined-metric

Conversation

@riderx
Copy link
Member

@riderx riderx commented Feb 9, 2026

Summary (AI generated)

Exclude invite-created accounts from onboarding registration metrics by introducing public.users.created_via_invite.

Motivation (AI generated)

“User Joined” and daily registrations are currently inflated by invited members (non-technical users, users joining an org that already has apps, and existing users joining additional orgs).

Business Impact (AI generated)

Improves accuracy of onboarding and growth metrics used for product decisions and reporting, without changing user-facing behavior.

Test Plan (AI generated)

  • bun lint:backend && bun lint
  • Manual (local Supabase):
    • Create a regular user row: INSERT INTO public.users (id, email) VALUES (gen_random_uuid(), 'self@test.com'); then verify created_via_invite = false.
    • Create an invite-created user row: INSERT INTO public.users (id, email, created_via_invite) VALUES (gen_random_uuid(), 'invited@test.com', true); then verify created_via_invite = true.

Screenshots (AI generated)

N/A

Checklist (AI generated)

  • My code follows the code style of this project and passes bun run lint:backend && bun run lint.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • My change has adequate E2E test coverage.
  • I have tested my code manually, and I have provided steps how to reproduce my tests

Generated with AI

Summary by CodeRabbit

  • New Features

    • Track whether a user was created via invitation vs. self-signup to improve analytics and signup behavior.
  • Bug Fixes / Resiliency

    • Rollout-safe handling so signup and analytics continue to work if the new flag isn't yet present; improved logging and retry behavior.
  • Tests

    • Added tests verifying the created_via_invite flag behavior.

Copilot AI review requested due to automatic review settings February 9, 2026 01:54
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 9, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds a new boolean column created_via_invite to public.users, updates DB migration and TypeScript types, and implements backward-compatible insertion and analytics changes across invitation handling and user-create triggers; includes tests for the new column behavior.

Changes

Cohort / File(s) Summary
Type Definitions
src/types/supabase.types.ts, supabase/functions/_backend/utils/supabase.types.ts
Add created_via_invite: boolean to Users.Row and created_via_invite?: boolean to Users.Insert/Users.Update.
Database Migration
supabase/migrations/20260209014020_user_created_via_invite.sql
Add created_via_invite boolean column to public.users with NOT NULL DEFAULT false and column comment.
Invitation Flow
supabase/functions/_backend/private/accept_invitation.ts
Pass context c into ensurePublicUserRowExists; include created_via_invite: true on public user inserts with rollout pattern: detect missing-column errors via isMissingCreatedViaInviteColumnError, log via cloudlog, and retry insert without the field if needed.
Analytics & Triggers
supabase/functions/_backend/triggers/logsnag_insights.ts, supabase/functions/_backend/triggers/on_user_create.ts
logsnag_insights: try filtered count excluding invite-created users, fall back to legacy count if column missing. on_user_create: only call LogSnag.track when created_via_invite is falsy and log failures.
Tests
tests/user-created-via-invite.test.ts
Add tests asserting default created_via_invite is false for normal inserts and true when explicitly inserted.

Sequence Diagram

sequenceDiagram
    participant User
    participant AcceptInvite as AcceptInvitationFn
    participant DB as SupabaseDB
    participant LogSnag as LogSnag
    participant CloudLog as CloudLog

    User->>AcceptInvite: accept invitation
    AcceptInvite->>DB: INSERT public.users (created_via_invite=true)
    alt column exists
        DB-->>AcceptInvite: success
    else column missing
        DB-->>AcceptInvite: error (missing column)
        AcceptInvite->>CloudLog: log missing-column error
        AcceptInvite->>DB: INSERT public.users (without created_via_invite)
        DB-->>AcceptInvite: success
    end
    alt created_via_invite is falsy
        AcceptInvite->>LogSnag: track signup event
        LogSnag-->>AcceptInvite: ack
    else created_via_invite is true
        Note right of AcceptInvite: skip LogSnag.track for invite signups
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I nibbled a flag into the row,
True for invites, false for those who grow,
If the column hides, I retry with care,
Counting signups rightly, logging where to repair,
A carrot-coded cheer — hop, hooray, we’re aware! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely reflects the main objective of excluding invited users from the 'User Joined' metric.
Description check ✅ Passed The description covers all required sections with adequate detail: summary explains the change, test plan provides specific commands and manual steps, and a checklist is included.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch riderx/user-joined-metric

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a users.created_via_invite flag to distinguish invite-created accounts from self-signups, then uses it to exclude invited users from the “User Joined” LogSnag event and the daily registration metric.

Changes:

  • Add created_via_invite boolean column on public.users (default false) with documentation comment.
  • Set created_via_invite = true when creating the public.users row via /private/accept_invitation (with a backward-compatible fallback).
  • Filter registers_today and gate “User Joined” tracking based on created_via_invite.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
supabase/migrations/20260209014020_user_created_via_invite.sql Adds users.created_via_invite column + column comment for metrics semantics.
supabase/functions/_backend/utils/supabase.types.ts Updates backend-generated Supabase DB types to include created_via_invite.
supabase/functions/_backend/triggers/on_user_create.ts Skips “User Joined” LogSnag event for invite-created users.
supabase/functions/_backend/triggers/logsnag_insights.ts Excludes invite-created users from registers_today metric with fallback behavior.
supabase/functions/_backend/private/accept_invitation.ts Marks users created through invite acceptance with created_via_invite: true (with fallback).
src/types/supabase.types.ts Updates frontend-generated Supabase DB types to include created_via_invite.

icon: '🎉',
user_id: record.id,
notify: false,
}).catch()
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promise.prototype.catch() without a rejection handler does not swallow errors; await LogSnag.track(...).catch() will still throw if track() rejects. If the intent is best-effort analytics, provide an explicit handler (optionally logging) so a LogSnag outage doesn't fail the user-create trigger.

Suggested change
}).catch()
}).catch((error) => {
cloudlog({
requestId: c.get('requestId'),
message: 'LogSnag.track user-register failed',
error,
})
})

Copilot uses AI. Check for mistakes.
Comment on lines 136 to 142
let { error: insertError } = await supabaseAdmin.from('users').insert(insertPayload)

// Backward compatible rollout: if the column doesn't exist yet, retry without it.
if (insertError?.message?.toLowerCase().includes('created_via_invite')) {
const { created_via_invite: _createdViaInvite, ...fallbackPayload } = insertPayload
;({ error: insertError } = await supabaseAdmin.from('users').insert(fallbackPayload))
}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The backward-compat fallback (retry insert without created_via_invite) is silent. When this triggers, it changes metrics semantics and makes it hard to diagnose schema/cache mismatches; consider logging the original insertError (at least once / at debug level) before retrying.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Comment on lines 429 to 435
} = await supabaseAdmin.from('users').insert(insertUserPayload).select().single()

// Backward compatible rollout: if the column doesn't exist yet, retry without it.
if (userNormalTableError?.message?.toLowerCase().includes('created_via_invite')) {
const { created_via_invite: _createdViaInvite, ...fallbackPayload } = insertUserPayload
;({ error: userNormalTableError, data } = await supabaseAdmin.from('users').insert(fallbackPayload).select().single())
}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: the silent fallback on created_via_invite insert errors makes it difficult to detect when invited users are being recorded without the flag. Logging the initial userNormalTableError before retrying would improve observability during rollout.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Comment on lines 418 to 426
if (filtered.error?.message?.toLowerCase().includes('created_via_invite')) {
const legacy = await supabase
.from('users')
.select('id', { count: 'exact', head: true })
.gte('created_at', last24h)
if (legacy.error)
cloudlog({ requestId: c.get('requestId'), message: 'registers_today legacy error', error: legacy.error })
return legacy.count ?? 0
}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When falling back to the legacy registers_today count due to a missing/unknown created_via_invite column, the triggering filtered.error isn’t logged. Logging that error (in addition to any legacy error) would help spot schema/cache rollouts where the filtered metric is temporarily unavailable.

Copilot uses AI. Check for mistakes.
Comment on lines 416 to 435
const insertUserPayload = {
id: user.user.id,
email: invitation.email,
first_name: invitation.first_name,
last_name: invitation.last_name,
enable_notifications: true,
opt_for_newsletters: body.opt_for_newsletters,
}).select().single()
created_via_invite: true,
}

let {
error: userNormalTableError,
data,
} = await supabaseAdmin.from('users').insert(insertUserPayload).select().single()

// Backward compatible rollout: if the column doesn't exist yet, retry without it.
if (userNormalTableError?.message?.toLowerCase().includes('created_via_invite')) {
const { created_via_invite: _createdViaInvite, ...fallbackPayload } = insertUserPayload
;({ error: userNormalTableError, data } = await supabaseAdmin.from('users').insert(fallbackPayload).select().single())
}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces new persisted behavior (marking invite-created accounts via created_via_invite) that affects onboarding metrics. There are tests for /private/accept_invitation error cases, but no test covering the successful path asserting the users.created_via_invite flag is set (and remains false for normal signups). Adding a success-path test would prevent regressions.

Copilot generated this review using guidance from repository custom instructions.
@riderx
Copy link
Member Author

riderx commented Feb 9, 2026

Addressed review comments (AI generated):

  • on_user_create: LogSnag tracking is now best-effort with an explicit .catch((error) => cloudlog(...)) so outages don’t fail the trigger.
  • accept_invitation: when we hit the backward-compat retry (missing created_via_invite column), we now cloudlog the original error before retrying.
  • logsnag_insights: if we fall back to the legacy registers_today count, we now log the triggering filtered error as well.
  • tests: added tests/user-created-via-invite.test.ts to assert the new column defaults to false and can be set true.

@riderx
Copy link
Member Author

riderx commented Feb 9, 2026

Follow-up on CodeRabbit notes (AI generated):

  • Added a TODO in logsnag_insights.ts to remove the backward-compat fallback once migration 20260209014020 is fully deployed.
  • Hardened the fallback checks to key off error codes (PGRST204 / 42703) in addition to message substring matching.
  • Updated PR description to include Motivation/Business Impact plus concrete manual verification SQL.

Docstring-coverage warning appears to be a CodeRabbit pre-merge check artifact (this PR doesn’t add new exported APIs); no repo CI check is failing on that.

Copy link
Contributor

Copilot AI commented Feb 9, 2026

@riderx I've opened a new pull request, #1606, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Contributor

Copilot AI commented Feb 9, 2026

@riderx I've opened a new pull request, #1607, to work on those changes. Once the pull request is ready, I'll request review from you.

…1607)

* Initial plan

* feat: log all initial insert errors before retry decision

Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>
@riderx riderx merged commit 217b00e into main Feb 9, 2026
8 of 9 checks passed
@riderx riderx deleted the riderx/user-joined-metric branch February 9, 2026 03:15
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 9, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants